Canonicalize bare-user-only perms with 0755 mask

For the flatpak use case where bare-user-only was introduced, we actually
don't want to support s{u,g} id files in particular.

Actually, I can't think of a reason to have anything outside of the
`0755 i.e. (u=rwx,g=rx,o=rx)` mask, so that's what we do here.

This will have the effect of treating existing `bare-user-only` repositories as
corrupted if they have files outside of that mask, but I think we should do this
now; most of the flatpak users will still be on `bare-user`, and we haven't
changed the semantics of that mode yet.

Note that in this patch we will also *reject* file content that doesn't
match this.  This is somewhat asymmetric, since we aren't similarly rejecting
e.g. directory metadata.  But, this will close off the biggest source
of the problem for flatpak (setuid binaries).

See: https://github.com/ostreedev/ostree/pull/908
See: https://github.com/flatpak/flatpak/pull/837

Closes: #909
Approved by: alexlarsson

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index 870a06c71be3a92e90d22a336a95a4ee9f2b3429..3ecea29da300be759732be38a882754298d48d62 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -281,7 +281,7 @@ commit_loose_object_trusted (OstreeRepo        *self,
         }
 
       if (objtype == OSTREE_OBJECT_TYPE_FILE &&
-          (self->mode == OSTREE_REPO_MODE_BARE_USER || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY))
+          self->mode == OSTREE_REPO_MODE_BARE_USER)
         {
           if (!object_is_symlink)
             {
@@ -294,10 +294,21 @@ commit_loose_object_trusted (OstreeRepo        *self,
                 return glnx_throw_errno (error);
             }
 
-          if (self->mode == OSTREE_REPO_MODE_BARE_USER &&
-              !write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error))
+          if (!write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error))
             return FALSE;
         }
+      else if (objtype == OSTREE_OBJECT_TYPE_FILE &&
+               self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY
+               && !object_is_symlink)
+        {
+          guint32 invalid_modebits = (mode & ~S_IFMT) & ~0755;
+          if (invalid_modebits > 0)
+            return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository",
+                                   mode, invalid_modebits);
+
+          if (fchmod (fd, mode) < 0)
+            return glnx_throw_errno_prefix (error, "fchmod");
+        }
 
       if (objtype == OSTREE_OBJECT_TYPE_FILE && _ostree_repo_mode_is_bare (self->mode))
         {
@@ -2293,11 +2304,23 @@ _ostree_repo_commit_modifier_apply (OstreeRepo               *self,
 
   if ((modifier->flags & OSTREE_REPO_COMMIT_MODIFIER_FLAGS_CANONICAL_PERMISSIONS) != 0)
     {
-
-      if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
+      guint mode = g_file_info_get_attribute_uint32 (modified_info, "unix::mode");
+      switch (g_file_info_get_file_type (file_info))
         {
-          guint current_mode = g_file_info_get_attribute_uint32 (modified_info, "unix::mode");
-          g_file_info_set_attribute_uint32 (modified_info, "unix::mode", current_mode | 0744);
+        case G_FILE_TYPE_REGULAR:
+          /* In particular, we want to squash the s{ug}id bits, but this also
+           * catches the sticky bit for example.
+           */
+          g_file_info_set_attribute_uint32 (modified_info, "unix::mode", mode & (S_IFREG | 0755));
+          break;
+        case G_FILE_TYPE_DIRECTORY:
+          /* Like the above but for directories */
+          g_file_info_set_attribute_uint32 (modified_info, "unix::mode", mode & (S_IFDIR | 0755));
+          break;
+        case G_FILE_TYPE_SYMBOLIC_LINK:
+          break;
+        default:
+          g_assert_not_reached ();
         }
       g_file_info_set_attribute_uint32 (modified_info, "unix::uid", 0);
       g_file_info_set_attribute_uint32 (modified_info, "unix::gid", 0);

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index d56bcc62eaa1ba9c310bf2e823d0b97e76a6019c..829452440da51504a4e3d1cb9f4fb79947d352ac 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -312,8 +312,13 @@ cd ${test_tmpdir}/checkout-test2-4
 $OSTREE commit ${COMMIT_ARGS} -b test2-override -s "with statoverride" --statoverride=../test-statoverride.txt
 cd ${test_tmpdir}
 $OSTREE checkout test2-override checkout-test2-override
-test -g checkout-test2-override/a/nested/2
-test -u checkout-test2-override/a/nested/3
+if ! is_bare_user_only_repo repo; then
+    test -g checkout-test2-override/a/nested/2
+    test -u checkout-test2-override/a/nested/3
+else
+    test '!' -g checkout-test2-override/a/nested/2
+    test '!' -u checkout-test2-override/a/nested/3
+fi
 echo "ok commit statoverride"
 
 cd ${test_tmpdir}